Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add sysroot lower bound to run_constrained #160

Closed
wants to merge 1 commit into from
Closed

add sysroot lower bound to run_constrained #160

wants to merge 1 commit into from

Conversation

minrk
Copy link
Member

@minrk minrk commented Jun 9, 2024

prevents downstream packages from building with older sysroot (#143)

draft because:

  • it's unclear if this is an openmpi-specific issue or should be done in sysroot's run_exports
  • may need repodata patches for prior 2.17 builds for solver consistency if this is the right thing to do
  • it seems like we need guidance from conda-forge core on how best to signal that "when build with sysroot X, linking against this package must also build with sysroot >=X", and when that's likely to be the case

@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@h-vetinari
Copy link
Member

it's unclear if this is an openmpi-specific issue or should be done in sysroot's run_exports

Given the discussion in conda-forge/linux-sysroot-feedstock#63, I'd suggest you add this to openmpi for now; if at any point it becomes added as a sysroot constraint itself, you can always remove it later.

@minrk minrk mentioned this pull request Jun 10, 2024
Open
@minrk
Copy link
Member Author

minrk commented Jun 10, 2024

Thanks! That's useful. If I've read that discussion correctly, it won't have the desired effect when openmpi is just in host, right? That limits the usefulness a bit, I suppose, since that's all linux-64 builds, which are the main ones affected by the current issue.

@minrk
Copy link
Member Author

minrk commented Jun 10, 2024 •
edited
Loading

I think adding -Wl,--allow-shlib-undefined to the wrapper ldflags as done in #159 is the fix for our situation (the tests do exercise building with 2.12), so we don't appear to actually have this transitive requirement. I think it remains unclear if/whether/how this transitive requirement may actually come up and how it should be expressed (seems impossible for host dependencies), but at least openmpi doesn't seem to have it.

I think it's worth adding -Wl,--allow-shlib-undefined as the solution to undefined reference to symbol@GLIBC_2.17, and mentioning that it's in $LDFLAGS to the conda-forge knowledge base somewhere.

@minrk minrk closed this Jun 10, 2024
@minrk minrk reopened this Jun 11, 2024
@minrk
Copy link
Member Author

minrk commented Jun 11, 2024

I changed my mind, on re-reading various situations from #143, adding this constraint will make it more likely for compilation to succeed in user environments, since building with compiler wrappers is something that's normal to occur for users at runtime, not just building packages in conda-build.

@minrk minrk mentioned this pull request Jun 11, 2024
Closed
1 task
@h-vetinari h-vetinari mentioned this pull request Jul 11, 2024
Open
@dalcinl
Copy link
Contributor

dalcinl commented Jul 30, 2024

@minrk What's the status of this PR? Are you waiting for anything to happen upstream in conda-forge? Or it is just an abandoned idea?

@minrk minrk mentioned this pull request Jul 31, 2024
Merged
3 tasks
@minrk
Copy link
Member Author

minrk commented Oct 1, 2024

I don't think this really makes sense, so I don't think we should do special things for openmpi here.

@minrk minrk closed this Oct 1, 2024
@minrk minrk deleted the run_constrained_sysroot branch October 1, 2024 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@astrofrog astrofrog Awaiting requested review from astrofrog astrofrog will be requested when the pull request is marked ready for review astrofrog is a code owner

@beckermr beckermr Awaiting requested review from beckermr beckermr will be requested when the pull request is marked ready for review beckermr is a code owner

@bekozi bekozi Awaiting requested review from bekozi bekozi will be requested when the pull request is marked ready for review bekozi is a code owner

@dalcinl dalcinl Awaiting requested review from dalcinl dalcinl will be requested when the pull request is marked ready for review dalcinl is a code owner

@j34ni j34ni Awaiting requested review from j34ni j34ni will be requested when the pull request is marked ready for review j34ni is a code owner

@leofang leofang Awaiting requested review from leofang leofang will be requested when the pull request is marked ready for review leofang is a code owner

@msarahan msarahan Awaiting requested review from msarahan msarahan will be requested when the pull request is marked ready for review msarahan is a code owner

@ocefpaf ocefpaf Awaiting requested review from ocefpaf ocefpaf will be requested when the pull request is marked ready for review ocefpaf is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@minrk @h-vetinari @dalcinl